🛡️ Sentinel: [HIGH] Fix unsafe path handling#423
Conversation
This patch addresses an insecure directory permissions handling in `Sources/Cacheout/Cleaner/CacheCleaner.swift`. The previous implementation relied solely on `FileManager.createDirectory(..., attributes: [.posixPermissions: 0o700])`, which is vulnerable to Time-Of-Check to Time-Of-Use (TOCTOU) symlink attacks as it only applies permissions to newly created directories and doesn't harden existing ones securely. The code has been updated to use safe POSIX APIs (`open` with `O_NOFOLLOW | O_DIRECTORY` and `fchmod`) to explicitly lock down the directory with 0700 permissions. Co-authored-by: acebytes <2820910+acebytes@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dec5338e85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| guard dirFd >= 0 else { return } | ||
| defer { close(dirFd) } | ||
| guard fchmod(dirFd, 0o700) == 0 else { return } |
There was a problem hiding this comment.
When an untrusted local process can rename entries in the user's home directory, this fd is only used for fchmod; the later open(logDir.appendingPathComponent(...)) re-walks ~/.cacheout, so a swap after this line can redirect the log append despite the hardening. Keep using dirFd for cleanup.log (for example with openat) so the file creation is anchored to the directory you just verified.
Useful? React with 👍 / 👎.
1 participant
🚨 Severity: HIGH
💡 Vulnerability: Found unsafe path handling when creating the log directory. High-level path operations can inadvertently follow unintended file system structures, leaving directories potentially insecure.
🎯 Impact: An attacker could potentially redirect file operations to unauthorized locations or bypass intended directory permissions.
🔧 Fix: Add defense-in-depth hardening to the directory by explicitly verifying the path and applying secure permissions securely without relying on high-level path resolution.
✅ Verification: Verify that the log directory is consistently created or updated with secure permissions and resilient against path redirection.
PR created automatically by Jules for task 5291456018300749461 started by @acebytes